Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gcoap: fix underflow when correcting ETag from cache [backport 2023.10] #19987

Merged
merged 1 commit into from
Oct 19, 2023
Merged

gcoap: fix underflow when correcting ETag from cache [backport 2023.10] #19987

merged 1 commit into from
Oct 19, 2023

Conversation

MrKevinWeiss
Copy link
Contributor

Backport of #19968

Contribution description

Fixes an underflow error when getting a new ETag from cache, by returning early.

Testing procedure

I repeated the testing procedures from #17801 (comment) (attention, the expected input for examples/gcoap changed slightly in the last year and a half)

diff --git a/aiocoap/cli/fileserver.py b/aiocoap/cli/fileserver.py
index 6af2b84..f5a9e71 100644
--- a/aiocoap/cli/fileserver.py
+++ b/aiocoap/cli/fileserver.py
@@ -154,6 +154,7 @@ class FileServer(Resource, aiocoap.interfaces.ObservableResource):
             elif S_ISREG(st.st_mode):
                 response = await self.render_get_file(request, path)
 
+        response.opt.max_age = 10
         response.opt.etag = etag
         return response

)

$ sudo ./dist/tool/tapsetup/tapsetup
$ PORT=tap1 USEMODULE="gcoap_forward_proxy nanocoap_cache" make -C examples/gcoap -j flash term
...
> ifconfig
ifconfig
Iface  6  HWaddr: DA:27:1D:A8:64:24 
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::d827:1dff:fea8:6424  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ffa8:6424

> 
[second terminal in aiocoap repo]
$ ip addr show dev tapbr0 scope link
5: tapbr0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
    link/ether de:1a:a8:09:45:b3 brd ff:ff:ff:ff:ff:ff
    inet6 fe80::dc1a:a8ff:fe09:45b3/64 scope link proto kernel_ll 
       valid_lft forever preferred_lft forever
$ mkdir -p test
$ echo "foobar" > test/a.txt
$ ./aiocoap-fileserver --bind "[$(ip addr show dev tapbr0 scope link | grep -o 'inet6 [0-9a-f:]\+' | sed 's/inet6 //')%tapbr0]" test/
...
[third terminal]
$ make -C examples/gcoap term
All up, running the shell now
> coap proxy set [fe80::d827:1dff:fea8:6424]:5683
coap proxy set [fe80::d827:1dff:fea8:6424]:5683
> coap get [fe80::dc1a:a8ff:fe09:45b3]:5683 /a.txt
coap get [fe80::dc1a:a8ff:fe09:45b3]:5683 /a.txt
gcoap_cli: sending msg ID 17604, 63 bytes
> --- blockwise start ---
gcoap: response Success, code 2.05, 7 bytes
foobar

--- blockwise complete ---
coap get [fe80::dc1a:a8ff:fe09:45b3]:5683 /a.txt
coap get [fe80::dc1a:a8ff:fe09:45b3]:5683 /a.txt
gcoap_cli: sending msg ID 17605, 63 bytes
--- blockwise start ---
gcoap: response Success, code 2.05, 7 bytes
foobar

--- blockwise complete ---

[wait >10 sec]

> coap get [fe80::dc1a:a8ff:fe09:45b3]:5683 /a.txt
coap get [fe80::dc1a:a8ff:fe09:45b3]:5683 /a.txt
gcoap_cli: sending msg ID 17606, 63 bytes
> --- blockwise start ---
gcoap: response Success, code 2.05, 7 bytes
foobar

--- blockwise complete ---
coap get [fe80::dc1a:a8ff:fe09:45b3]:5683 /a.txt
coap get [fe80::dc1a:a8ff:fe09:45b3]:5683 /a.txt
gcoap_cli: sending msg ID 17607, 63 bytes
--- blockwise start ---
gcoap: response Success, code 2.05, 7 bytes
foobar

--- blockwise complete ---
> ^C
native: exiting

Sniffing on tapbr0 should show something like.

image

Issues/PRs references

Reported off-band.

@MrKevinWeiss MrKevinWeiss added Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Oct 18, 2023
@riot-ci
Copy link

riot-ci commented Oct 18, 2023

Murdock results

✔️ PASSED

78124f2 gcoap: fix underflow when correcting ETag from cache

Success Failures Total Runtime
7937 0 7937 16m:33s

Artifacts

miri64
miri64 approved these changes Oct 18, 2023
View reviewed changes
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors merge

bors bot added a commit that referenced this pull request Oct 18, 2023
@bors @miri64
Merge #19987
bab7208 
19987: gcoap: fix underflow when correcting ETag from cache [backport 2023.10] r=miri64 a=MrKevinWeiss

# Backport of #19968



Co-authored-by: Martine Lenders <m.lenders@fu-berlin.de>
@bors
Copy link
Contributor

bors bot commented Oct 18, 2023

Build failed:

  • static-tests

@miri64
Copy link
Member

miri64 commented Oct 19, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Oct 19, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 39765d2 into RIOT-OS:2023.10-branch Oct 19, 2023
30 checks passed
@MrKevinWeiss MrKevinWeiss deleted the backport/2023.10/gcoap/fix/underflow branch October 20, 2023 07:32
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.10 milestone Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miri64 miri64 miri64 approved these changes

@haukepetersen haukepetersen Awaiting requested review from haukepetersen haukepetersen is a code owner

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann PeterKietzmann is a code owner

@cgundogan cgundogan Awaiting requested review from cgundogan cgundogan is a code owner

Assignees
No one assigned
Labels
Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
Release 2023.10
Development

Successfully merging this pull request may close these issues.
3 participants
@MrKevinWeiss @riot-ci @miri64